lower literal patterns #3805
lower literal patterns #3805
Conversation
|
Hint: break some logic in the function you want to find tests for and look at which tests failed. |
|
Huh! I guess it didn't matter in most situations, since we'll usually know the type from the match expression anyway. Lowering is usually tested through the type inference tests, and since type inference for literal patterns isn't implemented either, I assume there are no tests for it.
Probably; the actual literal value just doesn't matter for any of the analysis we do so far. LGTM, just one small suggestion. I think we can just merge this for now, and properly test it once we have the inference part implemented. bors d+ |
|
✌️ JoshMcguigan can now approve this pull request. To approve and merge a pull request, simply reply with |
JoshMcguigan
force-pushed
the
lower-literal-pat
branch
from
f37fd03 to
a9d0c9b
Compare
|
bors r+ |
Build succeeded |
3806: lower bool literal value r=flodiebold a=JoshMcguigan Following up on #3805, this PR adds the literal value to `ast::LiteralKind` so when we lower we can use the actual value from the source code rather than the default value for the type. Ultimately I plan to use this for exhaustiveness checking in #3706. I didn't include this in the previous PR because I wasn't sure if it made sense to add this information to `ast::LiteralKind` or provide some other mechanism to get this from `ast::Literal`. For now I've only implemented this for boolean literals, but I think it could be easily extended to other types. A possible exception to this are string literals, since we may not want to clone around an owned string to hold onto in `ast::LiteralKind`, and it'd be nice to avoid adding a generic lifetime as well. Perhaps we won't ever care about the actual value of a string literal? Co-authored-by: Josh Mcguigan <joshmcg88@gmail.com>
3 participants
While working on #3706 I discovered literal patterns weren't being lowered. This PR implements that lowering.
Questions for reviewers:
ast::LiteralKindtoLiteral, butast::LiteralKinddoesn't include information about the actual value of the literal, which causesLiteralto be created with the default value for the type (rather than the actual value in the source code). Am I correct in thinking that we'd eventually want to change things in such a way that we could initialize theLiteralwith the actual literal value? Is there an existing issue for this, or else perhaps I should create one to discuss how it should be implemented? My main question would be whetherast::LiteralKindshould be extended to hold the actual value, or if we should provide some other way to get that information fromast::Literal?lit.literal()returns anOption. Is returning aPat::Missingin theNonecase the right thing to do?ast::Pat::LiteralPattoPat::Lit. I don't have an immediate question here, but I just wanted to ensure this section is looked at closely during review.